-
Notifications
You must be signed in to change notification settings - Fork 307
fix(number-animation): add version information for the number animation component and fix the loading error problem under the SaaS theme #3390
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe updates introduce new and improved styling for the NumberAnimation component by adding relevant LESS files and imports. Metadata indicating the stability version is added to the NumberAnimation menu item. Additionally, fallback values are provided for several CSS variables in both NumberAnimation and Table components to ensure consistent styling defaults. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI Menu
participant NumberAnimation Component
participant Theme Styles
User->>UI Menu: Open menu
UI Menu->>UI Menu: Display NumberAnimation with meta info
User->>NumberAnimation Component: Select NumberAnimation
NumberAnimation Component->>Theme Styles: Apply updated styles with fallbacks
Theme Styles-->>NumberAnimation Component: Provide consistent appearance
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
examples/sites/demos/pc/menus.jsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-vue". (The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Walkthrough此PR为数值动画组件添加了版本信息,并修复了在SaaS主题下的加载错误问题。主要涉及对数值动画组件的元数据更新和样式文件的调整。 Changes
|
@@ -28,9 +28,9 @@ | |||
// 表格单元格字体大小 | |||
--tv-Table-td-font-size: var(--tv-font-size-default, 14px); | |||
// 表格边框颜色 | |||
--tv-Table-border-color: var(--tv-color-border-divider); | |||
--tv-Table-border-color: var(--tv-color-border-divider, #f0f0f0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change in --tv-Table-border-color
now includes a default value #f0f0f0
. Ensure that this default value does not conflict with existing themes or cause unintended visual changes.
// 表格复选框边框颜色 | ||
--tv-Table-icon-border-color: var(--tv-color-border); | ||
--tv-Table-icon-border-color: var(--tv-color-border, #c2c2c2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change in --tv-Table-icon-border-color
now includes a default value #c2c2c2
. Verify that this default value aligns with the design specifications and does not introduce visual inconsistencies.
WalkthroughThis PR adds version information to the numeric animation component and fixes loading errors under SaaS theme. It mainly involves metadata updates and style files adjustments of numerical animation components. Changes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
examples/sites/demos/pc/menus.js (1)
80-260
: 💡 Verification agent🧩 Analysis chain
Ensure demo route registration for NumberAnimation.
Adding a menu item is only part of the solution—please verify that a corresponding demo or route for
number-animation
has been registered in your router configuration (e.g.,examples/sites/demos/pc/routes.js
or wherever routes are declared). Without it, users clicking this menu entry will encounter a loading error.Run this script from the repo root to confirm the route exists:
🏁 Script executed:
#!/bin/bash # Search for NumberAnimation route/demo registration rg -n "number-animation" -R "examples/sites/demos/pc"Length of output: 456
Here’s a corrected verification script to search for any
number-animation
demo or route registration in theexamples/sites/demos/pc
directory:
🏁 Script executed:
#!/bin/bash # Recursively search for both dashed and PascalCase references rg -n "number-animation" examples/sites/demos/pc rg -n "NumberAnimation" examples/sites/demos/pcLength of output: 3581
Register NumberAnimation demo routes in your router config
We’ve confirmed that although the demo files live under
examples/sites/demos/pc/app/number-animation
, there’s no corresponding route entry fornumber-animation
in your router configuration. Without it, clicking “Number Animation” in the menu will 404.Please update your routes (e.g.
examples/sites/demos/pc/routes.js
) to include something like:{ path: '/number-animation', name: 'number-animation', component: () => import('./app/number-animation/basic-usage.vue'), children: [ { path: 'separator', name: 'number-animation-separator', component: () => import('./app/number-animation/separator.vue') }, { path: 'finish-events', name: 'number-animation-finish-events', component: () => import('./app/number-animation/finish-events.vue') }, { path: 'precision', name: 'number-animation-precision', component: () => import('./app/number-animation/precision.vue') } // add any other variants you want to expose ] }• File needing update:
examples/sites/demos/pc/routes.js
• Define the parent/number-animation
route pointing atbasic-usage.vue
• Register child routes for each demo variant (separator, finish-events, precision, etc.)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
examples/sites/demos/pc/menus.js
(1 hunks)packages/theme-saas/src/index.less
(1 hunks)packages/theme-saas/src/number-animation/index.less
(1 hunks)packages/theme/src/index.less
(1 hunks)packages/theme/src/number-animation/vars.less
(1 hunks)packages/theme/src/table/vars.less
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
🔇 Additional comments (5)
packages/theme-saas/src/index.less (1)
109-109
:✅ Verification successful
Import number-animation styles into SaaS theme
The new import for./number-animation/index.less
ensures the NumberAnimation component’s styles are included in the SaaS theme bundle. Placement betweennotify
andnumeric
is correct alphabetically.Run this script to verify the file exists in the expected location:
🏁 Script executed:
#!/bin/bash # Verify that the number-animation stylesheet is present if [ -f packages/theme-saas/src/number-animation/index.less ]; then echo "✅ packages/theme-saas/src/number-animation/index.less exists" else echo "❌ packages/theme-saas/src/number-animation/index.less is missing" exit 1 fiLength of output: 195
Import number-animation styles – Verified and Approved
- File:
packages/theme-saas/src/index.less
, line 109- The import
is correctly placed alphabetically between@import './number-animation/index.less';notify
andnumeric
.- Confirmed that
packages/theme-saas/src/number-animation/index.less
exists.No further changes required.
packages/theme/src/index.less (1)
91-91
:✅ Verification successful
Import number-animation styles into default theme
This import integrates the NumberAnimation styles into the core theme. The alphabetical placement betweennotify
andnumeric
is correct.Use this command to confirm the file is present:
🏁 Script executed:
#!/bin/bash # Verify that the number-animation stylesheet is present in the main theme if [ -f packages/theme/src/number-animation/index.less ]; then echo "✅ packages/theme/src/number-animation/index.less exists" else echo "❌ packages/theme/src/number-animation/index.less is missing" exit 1 fiLength of output: 180
✅ NumberAnimation styles successfully integrated into the core theme
The import atpackages/theme/src/index.less
is correctly placed alphabetically betweennotify
andnumeric
, andpackages/theme/src/number-animation/index.less
has been verified to exist.packages/theme/src/table/vars.less (1)
31-33
: Add fallback defaults for Table border colors
The new fallbacks (#f0f0f0
and#c2c2c2
) on--tv-Table-border-color
and--tv-Table-icon-border-color
ensure that table borders render consistently even if the corresponding CSS variables are undefined. These values align with existing theme defaults.packages/theme/src/number-animation/vars.less (1)
5-7
: Add fallback defaults for NumberAnimation font properties
The new fallbacks for--tv-NumberAnimation-font-weight
and--tv-NumberAnimation-font-size
ensure robust defaults if theme tokens are undefined. Please confirm that the design tokens--tv-font-weight-regular
and--tv-font-size-heading-lg
exist and match the intended defaults.examples/sites/demos/pc/menus.js (1)
253-260
: New stable version metadata added correctly.The
数值动画 (NumberAnimation)
menu entry now includes ameta.stable: "3.23.0"
field, matching the pattern used for other components likeVirtualScrollBox
andVirtualTree
.
@import '../custom.less'; | ||
|
||
@number-animation-item-prefix-cls: ~'@{css-prefix}number-animation'; | ||
|
||
.@{number-animation-item-prefix-cls} { | ||
@apply text-2xl; | ||
@apply font-normal; | ||
@apply mb-5; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Introduce NumberAnimation styling; add missing license header
The stylesheet correctly sets up the component prefix and applies utility classes (text-2xl
, font-normal
, mb-5
). However, to maintain consistency with other theme files, please add the standard MIT-style license header at the top. Also verify that @apply
utility classes and the global @{css-prefix}
variable are resolved by the build.
You can check for css-prefix
definition and @apply
support with:
🏁 Script executed:
#!/bin/bash
# Check that css-prefix is defined in custom.less
grep -R "css-prefix" -n packages/theme-saas/src/custom.less || echo "❌ css-prefix not found in custom.less"
# Ensure build supports @apply by inspecting PostCSS or Less plugin config (manually review)
Length of output: 1168
🏁 Script executed:
#!/bin/bash
# Search for PostCSS configuration
fd postcss.config.js || echo "no postcss.config.js found"
# Search for Tailwind configuration
fd tailwind.config.js || echo "no tailwind.config.js found"
# Check for @apply usage in theme-saas
rg "@apply" -n packages/theme-saas/src
Length of output: 110507
🏁 Script executed:
#!/bin/bash
# Search for existing license headers in theme-saas .less files
grep -R "MIT" -n packages/theme-saas/src || echo "No explicit 'MIT' mention found"
grep -R "license" -n packages/theme-saas/src
# Show top of a representative file to inspect header style
head -n 10 packages/theme-saas/src/checkbox/index.less
Length of output: 3222
Add MIT license header to number-animation/index.less
The @css-prefix
variable is defined in custom.less
and the PostCSS <code>@apply</code>
plugin is already in use across the theme, so only the license header is missing.
• File to update: packages/theme-saas/src/number-animation/index.less
• Copy the standard header from any existing Less file (e.g., date-table/index.less
) and prepend it above the imports.
Example snippet:
+ /*!
+ * Use of this source code is governed by an MIT-style license.
+ * See the LICENSE file in the project root for details.
+ */
@import '../custom.less';
@number-animation-item-prefix-cls: ~'@{css-prefix}number-animation';
.@{number-animation-item-prefix-cls} {
@apply text-2xl;
@apply font-normal;
@apply mb-5;
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
@import '../custom.less'; | |
@number-animation-item-prefix-cls: ~'@{css-prefix}number-animation'; | |
.@{number-animation-item-prefix-cls} { | |
@apply text-2xl; | |
@apply font-normal; | |
@apply mb-5; | |
} | |
/*! | |
* Use of this source code is governed by an MIT-style license. | |
* See the LICENSE file in the project root for details. | |
*/ | |
@import '../custom.less'; | |
@number-animation-item-prefix-cls: ~'@{css-prefix}number-animation'; | |
.@{number-animation-item-prefix-cls} { | |
@apply text-2xl; | |
@apply font-normal; | |
@apply mb-5; | |
} |
fix(number-animation): 为数值动画组件添加版本信息,修复SaaS主题下加载报错问题
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Style